Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(ui5-link): introduce interactiveAreaSize property #10762

Merged
merged 8 commits into from
Feb 10, 2025
Merged

feat(ui5-link): introduce interactiveAreaSize property #10762

merged 8 commits into from
Feb 10, 2025

Conversation

unazko
Copy link
Contributor

@unazko unazko commented Feb 4, 2025 •
edited
Loading

A new enumeration property called "interactiveAreaSize" is now added.
It is designed to make links easier to activate and helps meet the WCAG 2.2 Target Size requirement.
The property is applicable only for the SAP Horizon themes.
The interactive area should be sufficiently large to help users avoid accidentally selecting (clicking or tapping) on unintended UI elements.
There are two possible values for this property:
Normal and Large:

  • The default value is Normal and it implies no change in the visualization and behavior of the interactive elements.
  • The second option is Large. In this case the height of the interactive target area is increased. This option should be used when the interactive element is displayed as an overlay on top of other interactive parts of the page.

@unazko unazko requested a review from Stoev February 4, 2025 08:31
Stoev
Stoev reviewed Feb 4, 2025
View reviewed changes
Copy link

@Stoev Stoev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
As far as I've checked, the explanatory texts are absolutely identical to the corresponding ones in [FEATURE] sap.m.Link: Reactive area size API introduced. The commit header description follows correctly the convention.

@vladitasev
Copy link
Contributor

Officially, this feature seems to be called Target Size rather than Reactive Area Size:
https://www.w3.org/WAI/WCAG22/Understanding/target-size-minimum.html

In our internal Wiki it's also called Target Size, but there are a few mentions of "reactive area size".

Additionally, the values Inline and Overlay are confusing to me. From what I can see when you choose Overlay, the line-height is increased, which in itself has nothing to do with the term "overlay". Most likely the designers intended that you set Overlay when you want to use the link as an overlay or something to that effect, but overlay/inline are not intrinsic characteristics of the link itself, therefore IMO it would be wrong to set these names

I would personally understand more naturally something like:

  • property: targetAreaSize (or triggerAreaSize or interactiveAreaSize)
  • values: Normal (or Regular, Default) and Large (or Increased, or something like this)
    because I don't see how the word Overlay relates to the size of the touch area. It rather relates to the use case of the link when you want to set this value.

Let's discuss

@vladitasev
Copy link
Contributor

We settled on interactiveAreaSize: Normal and Large

@unazko unazko requested a review from Stoev February 6, 2025 12:10
vladitasev
vladitasev reviewed Feb 6, 2025
View reviewed changes
packages/main/src/Link.ts Outdated Show resolved Hide resolved
packages/main/src/Link.ts Show resolved Hide resolved
packages/main/src/Link.ts Outdated Show resolved Hide resolved
packages/main/src/Link.ts Outdated Show resolved Hide resolved
packages/main/src/types/InteractiveAreaSize.ts Outdated Show resolved Hide resolved
packages/main/src/types/InteractiveAreaSize.ts Outdated Show resolved Hide resolved
packages/main/src/types/InteractiveAreaSize.ts Outdated Show resolved Hide resolved
@vladitasev
Copy link
Contributor

vladitasev commented Feb 7, 2025 •
edited
Loading

Regarding the second note:

It is recommended to use theLarge value in use cases where the ui5-link component is used inside another interactive component such as a list item or a table cell, and the user may accidentally activate the underlying component when trying to activate the ui5-link. Setting the interactiveAreaSize property to Large increases ui5-link's invisible touch area, thus making it easier and more reliable to interact.

@Stoev
Copy link

Stoev commented Feb 7, 2025 •
edited
Loading

Hello, please consider a tentative suggestion for the second's note text, This is based on the Vladi's suggestion:

It is recommended to use the Large value in scenarios where the ui5-link component is placed inside another interactive component, such as a list item or a table cell, to improve ui5-link's reliability and usability.

Setting the interactiveAreaSize property to Large increases the ui5-link's invisible touch area. As a result, the user's intended one-time selection command is more likely to activate the desired ui5-link, with minimal chance of unintentionally activating the underlying component.

@unazko unazko requested a review from vladitasev February 7, 2025 13:36
Stoev
Stoev reviewed Feb 7, 2025
View reviewed changes
Copy link

@Stoev Stoev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Stoev
Stoev reviewed Feb 7, 2025
View reviewed changes
packages/main/src/Link.ts Outdated Show resolved Hide resolved
@unazko unazko requested a review from Stoev February 7, 2025 18:27
Stoev
Stoev reviewed Feb 10, 2025
View reviewed changes
Copy link

@Stoev Stoev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@unazko unazko merged commit 6ff54ce into SAP:main Feb 10, 2025
12 checks passed
@DMihaylova DMihaylova added this to the 2.8.0 milestone Feb 10, 2025
@ilhan007 ilhan007 changed the title feat(ui5-link): introduce reactive area size property feat(ui5-link): introduce interactiveAreaSize property Feb 10, 2025
dobromiraboycheva added a commit that referenced this pull request Feb 11, 2025
@dobromiraboycheva
Merge branch 'main' of https://github.com/SAP/ui5-webcomponents into …
cf8e3bd 
…ui5-settings

* 'main' of https://github.com/SAP/ui5-webcomponents:
  refactor(ui5-multi-combobox): switch to .tsx (#10807)
  refactor(ListItem): delete unused code (#10796)
  chore(deps-dev): bump esbuild from 0.19.9 to 0.25.0 (#10816)
  chore(deps-dev): bump vitest from 3.0.2 to 3.0.5 (#10785)
  test: add command for single cy test run (#10813)
  docs: fix TimelineGrowingMode enum docs (#10815)
  fix(ui5-side-navigation): remove inappropriate exclamation mark usage (#10769)
  feat(ui5-link): introduce reactive area size property (#10762)
  fix(ui5-side-navigation): "Space" key triggers links (#10767)
  chore(ui5-timeline): importing a timeline group item in the samples (#10793)
  chore: add ElementInternals shim (#10782)
  chore: fix MultiInput test (#10791)
  Translation Delivery (#10772)
  chore(ui5-select): remove unused css vars (#10789)
  chore: warn when scoping suffix is set too late (#10781)
  fix(ui5-button): remove tap highlight on safari (#10786)
  feat(ui5-table): action header cell is added (#10698)
  docs: fix List sample
  chore(release): publish v2.8.0-rc.0 [ci skip]
@ui5-webcomponents-bot
Copy link
Collaborator

🎉 This PR is included in version v2.8.0-rc.1 🎉

The release is available on v2.8.0-rc.1

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Stoev Stoev Stoev left review comments

@vladitasev vladitasev vladitasev approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
2.8.0
Development

Successfully merging this pull request may close these issues.
5 participants
@unazko @vladitasev @Stoev @ui5-webcomponents-bot @DMihaylova